Skip to content

Add testSelfSignedCertificateIsRejectedWithCorrectError #594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

dnadoba
Copy link
Collaborator

@dnadoba dnadoba commented Jun 15, 2022

We had bug report that an error during TLS handshake does not get correctly reported and instead a deadlineExceeded error is thrown. We have already fixed the issue in #588 and this PR just adds a test to verify the behavior.

Modifications:

  • add test testSelfSignedCertificateIsRejectedWithCorrectError which uses a self signed certificate to fail the TLS handshake
  • improve error description of NWTLSError. Previously a TLS error had a description that looked like this:
The operation couldn’t be completed. (Network.NWError error 2.)

which contains not enough for further investigation. It now contains the OSStatus code too which can be used to find out what actually went wrong:

-9808: Optional(bad certificate format)
  • make internal class NWErrorHandler final
  • fix error message of XCTAssertEqualTypeAndValue. It had the left hand side type and right hand side swapped.

Result

Improved test coverage and better error messages

@dnadoba dnadoba added the 🔨 semver/patch No public API change. label Jun 15, 2022
@dnadoba
Copy link
Collaborator Author

dnadoba commented Jun 17, 2022

5.7 & nightly fail because of Sendable warnings

@dnadoba dnadoba merged commit 794dc9d into swift-server:main Jun 17, 2022
@dnadoba dnadoba deleted the dn-self-signed-cert-error-test branch June 17, 2022 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants